Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ADD set command to apply settings #51

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

ADD set command to apply settings #51

wants to merge 2 commits into from

Conversation

bealdav
Copy link
Contributor

@bealdav bealdav commented Jul 1, 2019

@bealdav
Copy link
Contributor Author

bealdav commented Jul 5, 2019 •
edited
Loading

please @mourad-ehm @PierrickBrun could you have a look here ?

sbidoul
sbidoul reviewed Jul 5, 2019
View reviewed changes
Copy link
Member

@sbidoul sbidoul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds useful.

Roadmap improvements that come to mind would be an option to list existing settings with their description and current value, as well as a possibility to provide settings on the command line.

This will need tests and documentation update before merging.

setup.py Outdated
@@ -45,5 +45,6 @@
click-odoo-initdb=click_odoo_contrib.initdb:main
click-odoo-backupdb=click_odoo_contrib.backupdb:main
click-odoo-makepot=click_odoo_contrib.makepot:main
click-odoo-set=click_odoo_contrib.set:main
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The command name is a bit terse. I suppose it should mention what it sets, somehow.

@sbidoul sbidoul added the enhancement New feature or request label Jul 5, 2019
@bealdav
Copy link
Contributor Author

bealdav commented Jul 5, 2019

Thanks for your feedback Stéphane.
I'll go on with your advices

@sbidoul
Copy link
Member

sbidoul commented Jul 5, 2019

To be clear I'm not asking you to implement the roadmap ideas :) Only tests and doc update are required.

BTW, you can type "pre-commit install" once at the repo root on your local machine to auto check / fix the black/isort/flake8 issues on each commit.

@bealdav bealdav mentioned this pull request Jul 5, 2019
Closed
@sbidoul sbidoul added the WIP label Aug 30, 2019
@bealdav bealdav force-pushed the feat/generic-settings branch from 24cadc5 to a558918 Compare September 12, 2019 08:53
@bealdav
Copy link
Contributor Author

bealdav commented Sep 12, 2019

Hi @sbidoul do you have suggestions (existing test script or any) to make a useful test for this PR.
Do I need to test to a specific version of Odoo ?
Thanks

fcayre
fcayre approved these changes Sep 13, 2019
View reviewed changes
Copy link

@fcayre fcayre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (no functional test)

@blaggacao
Copy link
Contributor

It would be really nice to read a JSON/YAML file (optionally) and do merges on multiple sources.

This is useful as I might build a config tree for different Odoo runtimes. Some examples:

  • Different leaves as for memory settings on different Dev machines
  • Different leaves as for settings for worker settings when using a separate longpolling or Cron instance. (Basically, when not leaving the scheduling to Odoo)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sbidoul sbidoul sbidoul left review comments

@fcayre fcayre fcayre approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request WIP
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bealdav @sbidoul @blaggacao @fcayre